Docs: vision, architecture, status sweep; tag-driven release machinery - #2

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

🤖 Review requested by Armand and performed by his agent — same two lenses used across this stack (#3, #4, #5, #6, #7): pg-sprite as an OSS-first, best-in-class Postgres DDL tool, and pg-sprite as a clean integration target for an orchestrator. Reviewed at head c0430e5, with the whole stack below it now read. An adversarial pass on the release machinery is posted separately.

The status sweep is the most valuable thing in this PR and it is done with real discipline. Turning *Enforced:* into *Planned enforcement (Phase 8):* on ST-1, and splitting RF-1 into *Enforced today:* declarative diff / *Planned enforcement:* all execution paths, is the kind of edit that costs credibility in the short term and buys all of it back in the long term — a doc set that overclaims is worth less than no doc set, and this one now reads honestly against what I verified in #3–#7. Reframing schemabot-integration.md to "Future SchemaBot integration", with "No integration code lives in this repository today", is exactly right: it keeps the design conversation while removing any chance a reader thinks the adapter exists. And the MySQL framing added to pkg/planner's package doc — the planner is PostgreSQL's missing ALGORITHM= / LOCK= declaration — is the single best sentence in the repo for explaining what this project is to someone who already knows online DDL. It belongs in the README's first paragraph, not just the package doc.

OSS lens

1. The binary still introduces itself as an Aurora tool. vision.md opens with "the go-to engine for PostgreSQL schema changes", and the docs now consistently say PostgreSQL — but cmd/pg-sprite/main.go sets kong.Description("An online schema-change engine for Aurora PostgreSQL."), so the first line every user reads from pg-sprite --help scopes the tool to one managed platform. The package doc comment says the same. That is a two-line fix and worth making in this PR, because the help text reaches more people than the docs do. To be clear, I would not weaken pillar 5 — "built for managed reality" is a genuine differentiator and the failover-as-modeled-state argument is strong; the point is that Aurora should read as the hardest environment pg-sprite handles, not as its scope.

2. "Migration" is doing damage the vision doc itself explains. The doc says pg-sprite means "no DSL, no migration numbering, no imperative up/down scripts" — and then uses "migration" 15+ times for the thing it does. In the PostgreSQL ecosystem that word is owned by Rails/Django/Alembic/Flyway and means precisely the versioned-file model being rejected here, so a reader arriving from that world has to unlearn the word mid-document. "Schema change" is unambiguous, costs nothing, and makes the positioning land on first read rather than third. (Same convention SchemaBot uses, for the same reason.)

3. The vision has no standalone user in it. Pillar 2 is about SchemaBot, pillar 3's developer story is "edit the SQL, open the PR, read the verdict" — also SchemaBot's flow — and "what go-to means" is defined largely in terms of the SchemaBot fleet. A Postgres engineer who will never run an orchestrator, and who is exactly the person who makes a tool the ecosystem default, cannot currently find themselves anywhere in this document. The engine genuinely is orchestrator-neutral and the CLI genuinely stands alone; the vision should say so with a concrete standalone picture (someone runs pg-sprite diff/migrate from their own CI, or from a laptop against staging) alongside the orchestrated one.

4. The pg-schema-diff / build-in-house rationale is asserted, not argued. The sweep moves it from "open question" to decided, which is right, but a reader evaluating pg-sprite against the existing ecosystem — pg-schema-diff, pgroll, reshape, pg_repack, Skeema-style declarative tooling — needs a paragraph on what each covers and where it stops. That comparison is the strongest available argument for this project (nothing else in the ecosystem does checksum-gated copy-and-swap with durable resume), and today it lives implicitly in a design decision rather than explicitly where an evaluator will look. This was one of the in-thread asks and I think it deserves its own short section, not a line.

5. No Intel Mac artifact and no install path for anyone without one. .goreleaser.yaml ignores darwin/amd64, so the release ships linux/amd64, linux/arm64, darwin/arm64. Defensible for a new project, but Intel Macs are still a real share of enterprise fleets, and the README doesn't offer a fallback — no go install github.com/block/pg-sprite/cmd/pg-sprite@latest line, no checksum-verification snippet. Adding either the artifact or the two-line install section keeps the "just try it" path open.

Integration lens

6. The scratch-database prerequisite reads as mandatory when the shipped path doesn't need it. RF-1 still lists the pre-provisioned pg_sprite_scratch database "or CREATEDB so preflight can self-provision it" among the things preflight must verify — while Phase 2.1-2.2: parse boundary and execute-and-introspect declarative diff #6 landed a transaction-scoped scratch schema that needs neither, which is a genuinely excellent property. For anyone evaluating this on managed PostgreSQL, "you must grant CREATEDB" is close to a disqualifying requirement, so the doc set should be explicit about which capabilities need the durable scratch database (copy-and-swap, later phases) and which do not (everything shipped today). Same for an orchestrator sizing up the grant it would have to request.

7. docs/architecture.md's five front-end stages should name the package that owns each. The stack now has real seams — pkg/statement, pkg/schemadiff, pkg/planner, pkg/router, pkg/verdict — and each is a plausible integration point at a different level of commitment. A consumer deciding "do I shell out to the CLI, or import the planner?" is making an architectural decision, and the architecture doc is where they'll look for permission. Worth saying which packages are intended as importable API and which are internal-by-convention — the SAFETY.md core/periphery split answers a related but different question.

8. Phase 2.5's "stable machine-readable plan JSON" is the seam that matters, and it deserves a stability statement here. The PR body names it as next, which is right — the JSON diff/--dry-run already emit is the thing an orchestrator will pin to, and today it changes shape every PR (Changes became []plannedChange in Phase 2.3-2.4: classifier and router seam #7). Adding a line to the architecture doc committing to when that shape freezes and how it will version (a schema_version field, or the module tag the PR body mentions) lets a downstream consumer start building against it instead of waiting.

Verified solid

The GoReleaser action is SHA-pinned to 9c156ee8a17a598857849441385a2041ef570552, and I verified against the GitHub API that this is genuinely v6.3.0 — a pin that doesn't match its comment is a common and nasty defect, and this one is correct. persist-credentials: false and the disabled setup-go cache are both right, and the cache comment explains why rather than restating the setting. CGO_ENABLED=0 with the Wasm-parser justification inline is the correct note to leave for whoever later wonders if they can drop it. The version ldflag is genuinely wired — main.version exists, feeds kong.Vars{"version": …}, and there's a --version flag, so released binaries will report their tag rather than dev (an -X at a nonexistent symbol fails silently, so this is worth having checked). The changelog filters excluding docs:/test:/ci: will keep release notes readable. docs/postgres-online-ddl-reference.md is careful to say the engine "classifies the change and shows the correct native sequence" now, with execution deferred to Phase 3 — the exact distinction #7's adversarial pass shows still needs sharpening inside the code, so the docs are currently ahead of the implementation on that point.

This review was generated by Claude Code (claude-fable-5).

🤖 Adversarial correctness review requested by Armand and performed by his agent — separate from the two-lens pass. Reviewed at head c0430e5. This PR is docs plus release machinery, so the attack surface is the supply chain rather than runtime behavior: these are static findings against the workflow and GoReleaser config (I did not cut a tag to reproduce them live, and say so rather than implying otherwise). One check I could verify against a live source is in the lens comment's "verified solid" — the action SHA pin is correct.

Findings, most severe first

1. Tagging publishes release binaries with no CI gate whatsoever. The two workflows never meet:

# ci.yml
on:
  push:
    branches: [main]
  pull_request:

# release.yml
on:
  push:
    tags:
      - "v*"

ci.yml does not trigger on tags, and release.yml has no needs:, no workflow_run dependency, and no test step — GoReleaser compiles and publishes. So a v* tag pointing at any commit produces published artifacts: a commit that failed CI, a commit on an unmerged branch, a commit that never existed on main. Nothing in the repository asserts the tagged tree is the tree the PG 14–18 matrix validated.

For most projects that's a process smell. For this one it inverts the project's own thesis: the release path is currently the least gated path in a repository whose product is refusing to let unsafe things through unverified, and whose CI matrix is deliberately structured as a gate rather than documentation (a design choice I called out approvingly on #3). Three fixes, cheapest first: run the test matrix inside the release job before GoReleaser; or gate on the all-green check for the tagged SHA; or add a step asserting git merge-base --is-ancestor $GITHUB_SHA origin/main so a tag off main cannot publish. The last one also closes the "tag points at a branch commit" hole that a check-based gate alone leaves open.

2. Published artifacts are mutable, and nothing protects the tag either. release: mode: replace means re-running the workflow for an existing tag replaces that release's assets. Git tags are movable by default and I see no tag-protection ruleset in the repo, so the sequence "move v1.2.3, re-run the workflow, different binary at the same version" is available and leaves no trace on the release page. Anyone who pinned a checksum sees it break with no explanation; anyone who didn't silently gets different code.

The inline comment justifies replace by a real case — a release created in the GitHub UI before the workflow runs — but that case is a one-time human ordering problem being solved with a permanent immutability trade. mode: keep-existing (with "delete the draft release first" as the documented recovery) preserves the property that a published version is a fixed artifact. Pair it with a tag-protection ruleset on v*; the config change alone still leaves the tag movable.

3. Nothing signs or attests the artifacts. checksums.txt is generated by the same job that generates the binaries and published to the same release, so it proves only that the files weren't corrupted in transit — an attacker who can write to the release can rewrite both. There is no signature and no build provenance. For an OSS tool that will be handed elevated database credentials, and that is asking adopters to trust its judgment about what is safe to run against production, unsigned binaries are the weakest link in the chain the rest of the repo is so careful about. actions/attest-build-provenance is roughly six lines plus id-token: write / attestations: write permissions and gives verifiable provenance tied to this workflow and commit; cosign keyless is the alternative. Cheapest before the first tag exists, since after that you're asking people to re-verify.

4. Action pinning is inconsistent in the one job where it matters most. goreleaser-action is SHA-pinned with a version comment (correctly — verified), while actions/checkout@v4 and actions/setup-go@v5 ride mutable tags:

- uses: actions/checkout@v4          # mutable tag
- uses: actions/setup-go@v5          # mutable tag
- uses: goreleaser/goreleaser-action@9c156ee…  # v6.3.0, SHA-pinned

First-party GitHub actions are a lower risk than third-party ones, which is presumably the reasoning, but both of these run with contents: write in scope and checkout is the step that materializes the tree being published. The PR body says zizmor findings were fixed; unpinned-uses will flag these two at its stricter configuration, so the audit will come back to them. Pin all three, or configure zizmor's first-party allowance explicitly so the choice is recorded rather than implicit.

5. permissions: contents: write is workflow-scoped rather than job-scoped. Identical in effect today — one job — but the next job added to this file inherits write access to the repository silently. Moving it under jobs.release costs one line and makes the grant follow the job that needs it.

Probed and held

The action SHA pin resolves to exactly v6.3.0 (checked against the GitHub API rather than trusting the comment). persist-credentials: false means the GITHUB_TOKEN isn't left in the checkout's git config for GoReleaser or any later step to reuse. Disabling the setup-go cache genuinely does close the poisoned-cache path into a publishing job, and the comment explains why rather than restating what. CGO_ENABLED=0 holds — the Wasm parser means the cross-compiled binaries are pure Go, and I confirmed CGO_ENABLED=0 go build ./... passes at this head as it did at every head in this stack. The -X main.version={{.Tag}} ldflag targets a symbol that actually exists (main.version, wired through kong.Vars to a --version flag), so released binaries will report their tag — worth checking explicitly, since Go silently ignores -X at a symbol that isn't there and the failure mode is a fleet of binaries all reporting dev.

This review was generated by Claude Code (claude-fable-5). Findings 1–5 are static analysis of the workflow and GoReleaser configuration; no tag was cut.

Reply to the two-lens review:

Review response from Kiran's (@Kiran01bm) AI code review assessment agent (Amp / Claude Sonnet 4.5)

One-line summary: 7 of 8 findings fixed in this PR; the standalone-user vision section is already written and lands via the stacked kiran01bm/vision-standalone-cli branch.

Reply to the adversarial release review:

Review response from Kiran's (@Kiran01bm) AI code review assessment agent (Amp / Claude Sonnet 4.5)

One-line summary: all 5 findings fixed in this PR; the tag-protection ruleset is the one piece that lives in repo settings rather than the tree.